Skip to content

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Jun 20, 2022

fixes #77391

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 20, 2022
@rust-highfive
Copy link
Contributor

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2022
@TaKO8Ki TaKO8Ki force-pushed the point-to-type-param-definition branch from b6e51f0 to 0fc385b Compare June 20, 2022 15:43
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some general nits that I would appreciate if you fix up. The changes look fine though, and I like the diagnostic improvement.

@TaKO8Ki TaKO8Ki force-pushed the point-to-type-param-definition branch from 9dcc5d5 to 4a82fbb Compare June 21, 2022 07:50
@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki TaKO8Ki force-pushed the point-to-type-param-definition branch from 4a82fbb to 82e4e6d Compare June 21, 2022 08:52
@michaelwoerister
Copy link
Member

r? @compiler-errors, if that's OK.

Comment on lines 351 to 354
let generics = self.tcx.generics_of(
tcx.hir()
.body_owner_def_id(hir::BodyId { hir_id: self.body_id })
.to_def_id(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I overcomplicated this. Instead of tcx.hir().body_owner_def_id(..), let's just do self.body_id.owner.to_def_id() -- I've seen this being used elsewhere in typeck.

);
let type_param = generics.type_param(param_type, self.tcx);
Some((self.tcx.def_span(type_param.def_id), "for this"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a def-id, do you think it's useful to use self.def_kind(def_id).descr() so we can say "for this type parameter" or "for this enum" below?

Copy link
Member Author

@TaKO8Ki TaKO8Ki Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is useful. I will implement it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I am beginning to think it might be redundant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, why so? I don't know why this is more clear (current):

LL | struct Foo {
   |        --- method `clone` not found for this

Than this (with the change):

LL | struct Foo {
   |        --- method `clone` not found for this struct

The "this" feels incomplete.

Copy link
Member

@compiler-errors compiler-errors Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think at least we should be saying "for this type parameter" in this case, even if we don't add the def_kind(def_id).descr() in the ADT case below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I will implement it 👍

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jun 22, 2022

After the review, I will squash my commits.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, and I'm glad to see the code got simpler. I think "here" in the original message was vague.

Please squash then r=me

@compiler-errors
Copy link
Member

@bors delegate+

@bors
Copy link
Collaborator

bors commented Jun 22, 2022

✌️ @TaKO8Ki can now approve this pull request

…ssoc type

use `def_ident_span` , `body_owner_def_id` instead of `in_progress_typeck_results`, `guess_head_span`

use `body_id.owner` directly

add description to label
@TaKO8Ki TaKO8Ki force-pushed the point-to-type-param-definition branch from d7e4caa to 402dceb Compare June 22, 2022 04:42
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Jun 22, 2022

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Jun 22, 2022

📌 Commit 402dceb has been approved by compiler-errors

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2022
compiler-errors added a commit to compiler-errors/rust that referenced this pull request Jun 23, 2022
…ion, r=compiler-errors

Point to type parameter definition when not finding variant, method and associated item

fixes rust-lang#77391
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 25, 2022
…ion, r=compiler-errors

Point to type parameter definition when not finding variant, method and associated item

fixes rust-lang#77391
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 25, 2022
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#96412 (Windows: Iterative `remove_dir_all`)
 - rust-lang#98126 (Mitigate MMIO stale data vulnerability)
 - rust-lang#98149 (Set relocation_model to Pic on emscripten target)
 - rust-lang#98194 (Leak pthread_{mutex,rwlock}_t if it's dropped while locked.)
 - rust-lang#98298 (Point to type parameter definition when not finding variant, method and associated item)
 - rust-lang#98311 (Reverse folder hierarchy)
 - rust-lang#98401 (Add tracking issues to `--extern` option docs.)
 - rust-lang#98429 (Use correct substs in enum discriminant cast)
 - rust-lang#98431 (Suggest defining variable as mutable on `&mut _` type mismatch in pats)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8257ba2 into rust-lang:master Jun 25, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jun 25, 2022
@TaKO8Ki TaKO8Ki deleted the point-to-type-param-definition branch June 26, 2022 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a hint about type parameter conflicting with a structure name
7 participants